Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Movie Recommender solved (Bryan) #32

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

bryanidem
Copy link

You should add a movie.txt.gz file to the src/data folder, I tested my code using a provisional file src/data/movies_rapid_test.txt.gz

@vavimayor159
Copy link

vavimayor159 commented Apr 28, 2021

Please use your global .gitignore to avoid adding the .DS_Store files in the commits, here is a detailed guide on how to do it: https://www.freecodecamp.org/espanol/news/gitignore-explicado-que-es-y-como-agregar-a-tu-repositorio/.

Edit: Also include the .class files and the .lst files in the global .gitignore
Edit 2: Now that I think it twice, you should include all the files under the target directoy in the global .gitignore file.
Edit 3: Also include the .iml files in your global .gitignore this files are generated by the IDE and should not be included in your commits.

@@ -24,7 +33,5 @@ public void testDataInfo() throws IOException, TasteException {
assertThat(recommendations, hasItem("B0002O7Y8U"));
assertThat(recommendations, hasItem("B00004CQTF"));
assertThat(recommendations, hasItem("B000063W82"));

Copy link

@vavimayor159 vavimayor159 Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is usually a good idea to have just the necessary changes on the PR's, we should try not to include format changes unless is required. Avoiding them makes easier to review the PR's.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the advice! I'll restore the original file with only the necessary changes.

@@ -12,6 +12,8 @@

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>1.8</maven.compiler.source>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are this changes required? Are they part of the requirements?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! I've used maven, which allowed me to create a java project. In order to run, I had to add those dependencies.

Copy link

@vavimayor159 vavimayor159 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix issues mentioned above before continuing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants